Skip to content

[OpenMP][Flang] Emit default declare mappers implicitly for derived types #140562

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

TIFitis
Copy link
Member

@TIFitis TIFitis commented May 19, 2025

This patch adds support to emit default declare mappers for implicit mapping of derived types when not supplied by user. This especially helps tackle mapping of allocatables of derived types.

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp flang:semantics labels May 19, 2025
@llvmbot
Copy link
Member

llvmbot commented May 19, 2025

@llvm/pr-subscribers-github-workflow
@llvm/pr-subscribers-libcxx
@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-openmp

Author: Akash Banerjee (TIFitis)

Changes

This patch adds support to emit default declare mappers for implicit mapping of derived types when not supplied by user. This especially helps tackle mapping of allocatables of derived types.


Patch is 33.53 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/140562.diff

5 Files Affected:

  • (modified) flang/lib/Lower/OpenMP/ClauseProcessor.cpp (+23-16)
  • (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+131-1)
  • (modified) flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp (+1-1)
  • (modified) flang/lib/Semantics/resolve-names.cpp (+47-48)
  • (modified) flang/test/Lower/OpenMP/derived-type-map.f90 (+21-1)
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 8dcc8be9be5bf..cf25e91a437b8 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -1102,23 +1102,30 @@ void ClauseProcessor::processMapObjects(
 
   auto getDefaultMapperID = [&](const omp::Object &object,
                                 std::string &mapperIdName) {
-    if (!mlir::isa<mlir::omp::DeclareMapperOp>(
-            firOpBuilder.getRegion().getParentOp())) {
-      const semantics::DerivedTypeSpec *typeSpec = nullptr;
-
-      if (object.sym()->owner().IsDerivedType())
-        typeSpec = object.sym()->owner().derivedTypeSpec();
-      else if (object.sym()->GetType() &&
-               object.sym()->GetType()->category() ==
-                   semantics::DeclTypeSpec::TypeDerived)
-        typeSpec = &object.sym()->GetType()->derivedTypeSpec();
-
-      if (typeSpec) {
-        mapperIdName = typeSpec->name().ToString() + ".omp.default.mapper";
-        if (auto *sym = converter.getCurrentScope().FindSymbol(mapperIdName))
-          mapperIdName = converter.mangleName(mapperIdName, sym->owner());
-      }
+    const semantics::DerivedTypeSpec *typeSpec = nullptr;
+
+    if (object.sym()->GetType() && object.sym()->GetType()->category() ==
+                                       semantics::DeclTypeSpec::TypeDerived)
+      typeSpec = &object.sym()->GetType()->derivedTypeSpec();
+    else if (object.sym()->owner().IsDerivedType())
+      typeSpec = object.sym()->owner().derivedTypeSpec();
+
+    if (typeSpec) {
+      mapperIdName = typeSpec->name().ToString() + ".omp.default.mapper";
+      if (auto *sym = converter.getCurrentScope().FindSymbol(mapperIdName))
+        mapperIdName = converter.mangleName(mapperIdName, sym->owner());
+      else
+        mapperIdName =
+            converter.mangleName(mapperIdName, *typeSpec->GetScope());
     }
+
+    // Make sure we don't return a mapper to self
+    llvm::StringRef parentOpName;
+    if (auto declMapOp = mlir::dyn_cast<mlir::omp::DeclareMapperOp>(
+            firOpBuilder.getRegion().getParentOp()))
+      parentOpName = declMapOp.getSymName();
+    if (mapperIdName == parentOpName)
+      mapperIdName = "";
   };
 
   // Create the mapper symbol from its name, if specified.
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index cfcba0159db8d..8d5c26a4f2d58 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -2348,6 +2348,124 @@ genSingleOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
       queue, item, clauseOps);
 }
 
+static mlir::FlatSymbolRefAttr
+genImplicitDefaultDeclareMapper(lower::AbstractConverter &converter,
+                                mlir::Location loc, fir::RecordType recordType,
+                                llvm::StringRef mapperNameStr) {
+  fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+  lower::StatementContext stmtCtx;
+
+  // Save current insertion point before moving to the module scope to create
+  // the DeclareMapperOp
+  mlir::OpBuilder::InsertionGuard guard(firOpBuilder);
+
+  firOpBuilder.setInsertionPointToStart(converter.getModuleOp().getBody());
+  auto declMapperOp = firOpBuilder.create<mlir::omp::DeclareMapperOp>(
+      loc, mapperNameStr, recordType);
+  auto &region = declMapperOp.getRegion();
+  firOpBuilder.createBlock(&region);
+  auto mapperArg = region.addArgument(firOpBuilder.getRefType(recordType), loc);
+
+  auto declareOp =
+      firOpBuilder.create<hlfir::DeclareOp>(loc, mapperArg, /*uniq_name=*/"");
+
+  const auto genBoundsOps = [&](mlir::Value mapVal,
+                                llvm::SmallVectorImpl<mlir::Value> &bounds) {
+    fir::ExtendedValue extVal =
+        hlfir::translateToExtendedValue(mapVal.getLoc(), firOpBuilder,
+                                        hlfir::Entity{mapVal},
+                                        /*contiguousHint=*/true)
+            .first;
+    fir::factory::AddrAndBoundsInfo info = fir::factory::getDataOperandBaseAddr(
+        firOpBuilder, mapVal, /*isOptional=*/false, mapVal.getLoc());
+    bounds = fir::factory::genImplicitBoundsOps<mlir::omp::MapBoundsOp,
+                                                mlir::omp::MapBoundsType>(
+        firOpBuilder, info, extVal,
+        /*dataExvIsAssumedSize=*/false, mapVal.getLoc());
+  };
+
+  // Return a reference to the contents of a derived type with one field.
+  // Also return the field type.
+  const auto getFieldRef =
+      [&](mlir::Value rec,
+          unsigned index) -> std::tuple<mlir::Value, mlir::Type> {
+    auto recType = mlir::dyn_cast<fir::RecordType>(
+        fir::unwrapPassByRefType(rec.getType()));
+    auto [fieldName, fieldTy] = recType.getTypeList()[index];
+    mlir::Value field = firOpBuilder.create<fir::FieldIndexOp>(
+        loc, fir::FieldType::get(recType.getContext()), fieldName, recType,
+        fir::getTypeParams(rec));
+    return {firOpBuilder.create<fir::CoordinateOp>(
+                loc, firOpBuilder.getRefType(fieldTy), rec, field),
+            fieldTy};
+  };
+
+  mlir::omp::DeclareMapperInfoOperands clauseOps;
+  llvm::SmallVector<llvm::SmallVector<int64_t>> memberPlacementIndices;
+  llvm::SmallVector<mlir::Value> memberMapOps;
+
+  llvm::omp::OpenMPOffloadMappingFlags mapFlag =
+      llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO |
+      llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM |
+      llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_IMPLICIT;
+  mlir::omp::VariableCaptureKind captureKind =
+      mlir::omp::VariableCaptureKind::ByRef;
+  int64_t index = 0;
+
+  // Populate the declareMapper region with the map information.
+  for (const auto &[memberName, memberType] :
+       mlir::dyn_cast<fir::RecordType>(recordType).getTypeList()) {
+    auto [ref, type] = getFieldRef(declareOp.getBase(), index);
+    mlir::FlatSymbolRefAttr mapperId;
+    if (auto recType = mlir::dyn_cast<fir::RecordType>(memberType)) {
+      std::string mapperIdName =
+          recType.getName().str() + ".omp.default.mapper";
+      if (auto *sym = converter.getCurrentScope().FindSymbol(mapperIdName))
+        mapperIdName = converter.mangleName(mapperIdName, sym->owner());
+      else if (auto *sym = converter.getCurrentScope().FindSymbol(memberName))
+        mapperIdName = converter.mangleName(mapperIdName, sym->owner());
+
+      if (converter.getModuleOp().lookupSymbol(mapperIdName))
+        mapperId = mlir::FlatSymbolRefAttr::get(&converter.getMLIRContext(),
+                                                mapperIdName);
+      else
+        mapperId = genImplicitDefaultDeclareMapper(converter, loc, recType,
+                                                   mapperIdName);
+    }
+
+    llvm::SmallVector<mlir::Value> bounds;
+    genBoundsOps(ref, bounds);
+    mlir::Value mapOp = createMapInfoOp(
+        firOpBuilder, loc, ref, /*varPtrPtr=*/mlir::Value{}, "", bounds,
+        /*members=*/{},
+        /*membersIndex=*/mlir::ArrayAttr{},
+        static_cast<
+            std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
+            mapFlag),
+        captureKind, ref.getType(), /*partialMap=*/false, mapperId);
+    memberMapOps.emplace_back(mapOp);
+    memberPlacementIndices.emplace_back(llvm::SmallVector<int64_t>{index++});
+  }
+
+  llvm::SmallVector<mlir::Value> bounds;
+  genBoundsOps(declareOp.getOriginalBase(), bounds);
+  mlir::omp::MapInfoOp mapOp = createMapInfoOp(
+      firOpBuilder, loc, declareOp.getOriginalBase(),
+      /*varPtrPtr=*/mlir::Value(), /*name=*/"", bounds, memberMapOps,
+      firOpBuilder.create2DI64ArrayAttr(memberPlacementIndices),
+      static_cast<std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
+          mapFlag),
+      captureKind, declareOp.getType(0),
+      /*partialMap=*/true);
+
+  clauseOps.mapVars.emplace_back(mapOp);
+
+  firOpBuilder.create<mlir::omp::DeclareMapperInfoOp>(loc, clauseOps.mapVars);
+  // declMapperOp->dumpPretty();
+  return mlir::FlatSymbolRefAttr::get(&converter.getMLIRContext(),
+                                      mapperNameStr);
+}
+
 static mlir::omp::TargetOp
 genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
             lower::StatementContext &stmtCtx,
@@ -2420,15 +2538,26 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
       name << sym.name().ToString();
 
       mlir::FlatSymbolRefAttr mapperId;
-      if (sym.GetType()->category() == semantics::DeclTypeSpec::TypeDerived) {
+      if (sym.GetType()->category() == semantics::DeclTypeSpec::TypeDerived &&
+          defaultMaps.empty()) {
         auto &typeSpec = sym.GetType()->derivedTypeSpec();
         std::string mapperIdName =
             typeSpec.name().ToString() + ".omp.default.mapper";
         if (auto *sym = converter.getCurrentScope().FindSymbol(mapperIdName))
           mapperIdName = converter.mangleName(mapperIdName, sym->owner());
+        else
+          mapperIdName =
+              converter.mangleName(mapperIdName, *typeSpec.GetScope());
+
         if (converter.getModuleOp().lookupSymbol(mapperIdName))
           mapperId = mlir::FlatSymbolRefAttr::get(&converter.getMLIRContext(),
                                                   mapperIdName);
+        else
+          mapperId = genImplicitDefaultDeclareMapper(
+              converter, loc,
+              mlir::cast<fir::RecordType>(
+                  converter.genType(sym.GetType()->derivedTypeSpec())),
+              mapperIdName);
       }
 
       fir::factory::AddrAndBoundsInfo info =
@@ -4039,6 +4168,7 @@ genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
   ClauseProcessor cp(converter, semaCtx, clauses);
   cp.processMap(loc, stmtCtx, clauseOps);
   firOpBuilder.create<mlir::omp::DeclareMapperInfoOp>(loc, clauseOps.mapVars);
+  // declMapperOp->dumpPretty();
 }
 
 static void
diff --git a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
index 86095d708f7e5..1ef6239e51fa8 100644
--- a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
+++ b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
@@ -377,7 +377,7 @@ class MapInfoFinalizationPass
                                    getDescriptorMapType(mapType, target)),
             op.getMapCaptureTypeAttr(), /*varPtrPtr=*/mlir::Value{}, newMembers,
             newMembersAttr, /*bounds=*/mlir::SmallVector<mlir::Value>{},
-            /*mapperId*/ mlir::FlatSymbolRefAttr(), op.getNameAttr(),
+            op.getMapperIdAttr(), op.getNameAttr(),
             /*partial_map=*/builder.getBoolAttr(false));
     op.replaceAllUsesWith(newDescParentMapOp.getResult());
     op->erase();
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 322562b06b87f..318af99a57848 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -2163,7 +2163,7 @@ void AttrsVisitor::SetBindNameOn(Symbol &symbol) {
   }
   symbol.SetBindName(std::move(*label));
   if (!oldBindName.empty()) {
-    if (const std::string * newBindName{symbol.GetBindName()}) {
+    if (const std::string *newBindName{symbol.GetBindName()}) {
       if (oldBindName != *newBindName) {
         Say(symbol.name(),
             "The entity '%s' has multiple BIND names ('%s' and '%s')"_err_en_US,
@@ -2285,7 +2285,7 @@ void DeclTypeSpecVisitor::Post(const parser::TypeSpec &typeSpec) {
   // expression semantics if the DeclTypeSpec is a valid TypeSpec.
   // The grammar ensures that it's an intrinsic or derived type spec,
   // not TYPE(*) or CLASS(*) or CLASS(T).
-  if (const DeclTypeSpec * spec{state_.declTypeSpec}) {
+  if (const DeclTypeSpec *spec{state_.declTypeSpec}) {
     switch (spec->category()) {
     case DeclTypeSpec::Numeric:
     case DeclTypeSpec::Logical:
@@ -2293,7 +2293,7 @@ void DeclTypeSpecVisitor::Post(const parser::TypeSpec &typeSpec) {
       typeSpec.declTypeSpec = spec;
       break;
     case DeclTypeSpec::TypeDerived:
-      if (const DerivedTypeSpec * derived{spec->AsDerived()}) {
+      if (const DerivedTypeSpec *derived{spec->AsDerived()}) {
         CheckForAbstractType(derived->typeSymbol()); // C703
         typeSpec.declTypeSpec = spec;
       }
@@ -2802,8 +2802,8 @@ Symbol &ScopeHandler::MakeSymbol(const parser::Name &name, Attrs attrs) {
 Symbol &ScopeHandler::MakeHostAssocSymbol(
     const parser::Name &name, const Symbol &hostSymbol) {
   Symbol &symbol{*NonDerivedTypeScope()
-                      .try_emplace(name.source, HostAssocDetails{hostSymbol})
-                      .first->second};
+          .try_emplace(name.source, HostAssocDetails{hostSymbol})
+          .first->second};
   name.symbol = &symbol;
   symbol.attrs() = hostSymbol.attrs(); // TODO: except PRIVATE, PUBLIC?
   // These attributes can be redundantly reapplied without error
@@ -2891,7 +2891,7 @@ void ScopeHandler::ApplyImplicitRules(
   if (context().HasError(symbol) || !NeedsType(symbol)) {
     return;
   }
-  if (const DeclTypeSpec * type{GetImplicitType(symbol)}) {
+  if (const DeclTypeSpec *type{GetImplicitType(symbol)}) {
     if (!skipImplicitTyping_) {
       symbol.set(Symbol::Flag::Implicit);
       symbol.SetType(*type);
@@ -2991,7 +2991,7 @@ const DeclTypeSpec *ScopeHandler::GetImplicitType(
   const auto *type{implicitRulesMap_->at(scope).GetType(
       symbol.name(), respectImplicitNoneType)};
   if (type) {
-    if (const DerivedTypeSpec * derived{type->AsDerived()}) {
+    if (const DerivedTypeSpec *derived{type->AsDerived()}) {
       // Resolve any forward-referenced derived type; a quick no-op else.
       auto &instantiatable{*const_cast<DerivedTypeSpec *>(derived)};
       instantiatable.Instantiate(currScope());
@@ -3706,10 +3706,10 @@ void ModuleVisitor::DoAddUse(SourceName location, SourceName localName,
     } else if (IsPointer(p1) || IsPointer(p2)) {
       return false;
     } else if (const auto *subp{p1.detailsIf<SubprogramDetails>()};
-               subp && !subp->isInterface()) {
+        subp && !subp->isInterface()) {
       return false; // defined in module, not an external
     } else if (const auto *subp{p2.detailsIf<SubprogramDetails>()};
-               subp && !subp->isInterface()) {
+        subp && !subp->isInterface()) {
       return false; // defined in module, not an external
     } else {
       // Both are external interfaces, perhaps to the same procedure
@@ -3969,7 +3969,7 @@ Scope *ModuleVisitor::FindModule(const parser::Name &name,
   if (scope) {
     if (DoesScopeContain(scope, currScope())) { // 14.2.2(1)
       std::optional<SourceName> submoduleName;
-      if (const Scope * container{FindModuleOrSubmoduleContaining(currScope())};
+      if (const Scope *container{FindModuleOrSubmoduleContaining(currScope())};
           container && container->IsSubmodule()) {
         submoduleName = container->GetName();
       }
@@ -4074,7 +4074,7 @@ bool InterfaceVisitor::isAbstract() const {
 
 void InterfaceVisitor::AddSpecificProcs(
     const std::list<parser::Name> &names, ProcedureKind kind) {
-  if (Symbol * symbol{GetGenericInfo().symbol};
+  if (Symbol *symbol{GetGenericInfo().symbol};
       symbol && symbol->has<GenericDetails>()) {
     for (const auto &name : names) {
       specificsForGenericProcs_.emplace(symbol, std::make_pair(&name, kind));
@@ -4174,7 +4174,7 @@ void GenericHandler::DeclaredPossibleSpecificProc(Symbol &proc) {
 }
 
 void InterfaceVisitor::ResolveNewSpecifics() {
-  if (Symbol * generic{genericInfo_.top().symbol};
+  if (Symbol *generic{genericInfo_.top().symbol};
       generic && generic->has<GenericDetails>()) {
     ResolveSpecificsInGeneric(*generic, false);
   }
@@ -4259,7 +4259,7 @@ bool SubprogramVisitor::HandleStmtFunction(const parser::StmtFunctionStmt &x) {
           name.source);
       MakeSymbol(name, Attrs{}, UnknownDetails{});
     } else if (auto *entity{ultimate.detailsIf<EntityDetails>()};
-               entity && !ultimate.has<ProcEntityDetails>()) {
+        entity && !ultimate.has<ProcEntityDetails>()) {
       resultType = entity->type();
       ultimate.details() = UnknownDetails{}; // will be replaced below
     } else {
@@ -4315,7 +4315,7 @@ bool SubprogramVisitor::Pre(const parser::Suffix &suffix) {
     } else {
       Message &msg{Say(*suffix.resultName,
           "RESULT(%s) may appear only in a function"_err_en_US)};
-      if (const Symbol * subprogram{InclusiveScope().symbol()}) {
+      if (const Symbol *subprogram{InclusiveScope().symbol()}) {
         msg.Attach(subprogram->name(), "Containing subprogram"_en_US);
       }
     }
@@ -4833,7 +4833,7 @@ Symbol *ScopeHandler::FindSeparateModuleProcedureInterface(
       symbol = generic->specific();
     }
   }
-  if (const Symbol * defnIface{FindSeparateModuleSubprogramInterface(symbol)}) {
+  if (const Symbol *defnIface{FindSeparateModuleSubprogramInterface(symbol)}) {
     // Error recovery in case of multiple definitions
     symbol = const_cast<Symbol *>(defnIface);
   }
@@ -4970,8 +4970,8 @@ bool SubprogramVisitor::HandlePreviousCalls(
     return generic->specific() &&
         HandlePreviousCalls(name, *generic->specific(), subpFlag);
   } else if (const auto *proc{symbol.detailsIf<ProcEntityDetails>()}; proc &&
-             !proc->isDummy() &&
-             !symbol.attrs().HasAny(Attrs{Attr::INTRINSIC, Attr::POINTER})) {
+      !proc->isDummy() &&
+      !symbol.attrs().HasAny(Attrs{Attr::INTRINSIC, Attr::POINTER})) {
     // There's a symbol created for previous calls to this subprogram or
     // ENTRY's name.  We have to replace that symbol in situ to avoid the
     // obligation to rewrite symbol pointers in the parse tree.
@@ -5012,7 +5012,7 @@ void SubprogramVisitor::CheckExtantProc(
   if (auto *prev{FindSymbol(name)}) {
     if (IsDummy(*prev)) {
     } else if (auto *entity{prev->detailsIf<EntityDetails>()};
-               IsPointer(*prev) && entity && !entity->type()) {
+        IsPointer(*prev) && entity && !entity->type()) {
       // POINTER attribute set before interface
     } else if (inInterfaceBlock() && currScope() != prev->owner()) {
       // Procedures in an INTERFACE block do not resolve to symbols
@@ -5068,7 +5068,7 @@ Symbol &SubprogramVisitor::PushSubprogramScope(const parser::Name &name,
     }
     set_inheritFromParent(false); // interfaces don't inherit, even if MODULE
   }
-  if (Symbol * found{FindSymbol(name)};
+  if (Symbol *found{FindSymbol(name)};
       found && found->has<HostAssocDetails>()) {
     found->set(subpFlag); // PushScope() created symbol
   }
@@ -5910,9 +5910,9 @@ void DeclarationVisitor::Post(const parser::VectorTypeSpec &x) {
     vectorDerivedType.CookParameters(GetFoldingContext());
   }
 
-  if (const DeclTypeSpec *
-      extant{ppcBuiltinTypesScope->FindInstantiatedDerivedType(
-          vectorDerivedType, DeclTypeSpec::Category::TypeDerived)}) {
+  if (const DeclTypeSpec *extant{
+          ppcBuiltinTypesScope->FindInstantiatedDerivedType(
+              vectorDerivedType, DeclTypeSpec::Category::TypeDerived)}) {
     // This derived type and parameter expressions (if any) are already present
     // in the __ppc_intrinsics scope.
     SetDeclTypeSpec(*extant);
@@ -5934,7 +5934,7 @@ bool DeclarationVisitor::Pre(const parser::DeclarationTypeSpec::Type &) {
 
 void DeclarationVisitor::Post(const parser::DeclarationTypeSpec::Type &type) {
   const parser::Name &derivedName{std::get<parser::Name>(type.derived.t)};
-  if (const Symbol * derivedSymbol{derivedName.symbol}) {
+  if (const Symbol *derivedSymbol{derivedName.symbol}) {
     CheckForAbstractType(*derivedSymbol); // C706
   }
 }
@@ -6003,8 +6003,8 @@ void DeclarationVisitor::Post(const parser::DerivedTypeSpec &x) {
   if (!spec->MightBeParameterized()) {
     spec->EvaluateParameters(context());
   }
-  if (const DeclTypeSpec *
-      extant{currScope().FindInstantiatedDerivedType(*spec, category)}) {
+  if (const DeclTypeSpec *extant{
+          currScope().FindInstantiatedDerivedType(*spec, category)}) {
     // This derived type and parameter expressions (if any) are already present
     // in this scope.
     SetDeclTypeSpec(*extant);
@@ -6035,8 +6035,7 @@ void DeclarationVisitor::Post(const parser::DeclarationTypeSpec::Record &rec) {
   if (auto spec{ResolveDerivedType(typeName)}) {
     spec->CookParameters(GetFoldingContext());
   ...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented May 19, 2025

@llvm/pr-subscribers-flang-fir-hlfir

Author: Akash Banerjee (TIFitis)

Changes

This patch adds support to emit default declare mappers for implicit mapping of derived types when not supplied by user. This especially helps tackle mapping of allocatables of derived types.


Patch is 33.53 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/140562.diff

5 Files Affected:

  • (modified) flang/lib/Lower/OpenMP/ClauseProcessor.cpp (+23-16)
  • (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+131-1)
  • (modified) flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp (+1-1)
  • (modified) flang/lib/Semantics/resolve-names.cpp (+47-48)
  • (modified) flang/test/Lower/OpenMP/derived-type-map.f90 (+21-1)
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 8dcc8be9be5bf..cf25e91a437b8 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -1102,23 +1102,30 @@ void ClauseProcessor::processMapObjects(
 
   auto getDefaultMapperID = [&](const omp::Object &object,
                                 std::string &mapperIdName) {
-    if (!mlir::isa<mlir::omp::DeclareMapperOp>(
-            firOpBuilder.getRegion().getParentOp())) {
-      const semantics::DerivedTypeSpec *typeSpec = nullptr;
-
-      if (object.sym()->owner().IsDerivedType())
-        typeSpec = object.sym()->owner().derivedTypeSpec();
-      else if (object.sym()->GetType() &&
-               object.sym()->GetType()->category() ==
-                   semantics::DeclTypeSpec::TypeDerived)
-        typeSpec = &object.sym()->GetType()->derivedTypeSpec();
-
-      if (typeSpec) {
-        mapperIdName = typeSpec->name().ToString() + ".omp.default.mapper";
-        if (auto *sym = converter.getCurrentScope().FindSymbol(mapperIdName))
-          mapperIdName = converter.mangleName(mapperIdName, sym->owner());
-      }
+    const semantics::DerivedTypeSpec *typeSpec = nullptr;
+
+    if (object.sym()->GetType() && object.sym()->GetType()->category() ==
+                                       semantics::DeclTypeSpec::TypeDerived)
+      typeSpec = &object.sym()->GetType()->derivedTypeSpec();
+    else if (object.sym()->owner().IsDerivedType())
+      typeSpec = object.sym()->owner().derivedTypeSpec();
+
+    if (typeSpec) {
+      mapperIdName = typeSpec->name().ToString() + ".omp.default.mapper";
+      if (auto *sym = converter.getCurrentScope().FindSymbol(mapperIdName))
+        mapperIdName = converter.mangleName(mapperIdName, sym->owner());
+      else
+        mapperIdName =
+            converter.mangleName(mapperIdName, *typeSpec->GetScope());
     }
+
+    // Make sure we don't return a mapper to self
+    llvm::StringRef parentOpName;
+    if (auto declMapOp = mlir::dyn_cast<mlir::omp::DeclareMapperOp>(
+            firOpBuilder.getRegion().getParentOp()))
+      parentOpName = declMapOp.getSymName();
+    if (mapperIdName == parentOpName)
+      mapperIdName = "";
   };
 
   // Create the mapper symbol from its name, if specified.
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index cfcba0159db8d..8d5c26a4f2d58 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -2348,6 +2348,124 @@ genSingleOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
       queue, item, clauseOps);
 }
 
+static mlir::FlatSymbolRefAttr
+genImplicitDefaultDeclareMapper(lower::AbstractConverter &converter,
+                                mlir::Location loc, fir::RecordType recordType,
+                                llvm::StringRef mapperNameStr) {
+  fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+  lower::StatementContext stmtCtx;
+
+  // Save current insertion point before moving to the module scope to create
+  // the DeclareMapperOp
+  mlir::OpBuilder::InsertionGuard guard(firOpBuilder);
+
+  firOpBuilder.setInsertionPointToStart(converter.getModuleOp().getBody());
+  auto declMapperOp = firOpBuilder.create<mlir::omp::DeclareMapperOp>(
+      loc, mapperNameStr, recordType);
+  auto &region = declMapperOp.getRegion();
+  firOpBuilder.createBlock(&region);
+  auto mapperArg = region.addArgument(firOpBuilder.getRefType(recordType), loc);
+
+  auto declareOp =
+      firOpBuilder.create<hlfir::DeclareOp>(loc, mapperArg, /*uniq_name=*/"");
+
+  const auto genBoundsOps = [&](mlir::Value mapVal,
+                                llvm::SmallVectorImpl<mlir::Value> &bounds) {
+    fir::ExtendedValue extVal =
+        hlfir::translateToExtendedValue(mapVal.getLoc(), firOpBuilder,
+                                        hlfir::Entity{mapVal},
+                                        /*contiguousHint=*/true)
+            .first;
+    fir::factory::AddrAndBoundsInfo info = fir::factory::getDataOperandBaseAddr(
+        firOpBuilder, mapVal, /*isOptional=*/false, mapVal.getLoc());
+    bounds = fir::factory::genImplicitBoundsOps<mlir::omp::MapBoundsOp,
+                                                mlir::omp::MapBoundsType>(
+        firOpBuilder, info, extVal,
+        /*dataExvIsAssumedSize=*/false, mapVal.getLoc());
+  };
+
+  // Return a reference to the contents of a derived type with one field.
+  // Also return the field type.
+  const auto getFieldRef =
+      [&](mlir::Value rec,
+          unsigned index) -> std::tuple<mlir::Value, mlir::Type> {
+    auto recType = mlir::dyn_cast<fir::RecordType>(
+        fir::unwrapPassByRefType(rec.getType()));
+    auto [fieldName, fieldTy] = recType.getTypeList()[index];
+    mlir::Value field = firOpBuilder.create<fir::FieldIndexOp>(
+        loc, fir::FieldType::get(recType.getContext()), fieldName, recType,
+        fir::getTypeParams(rec));
+    return {firOpBuilder.create<fir::CoordinateOp>(
+                loc, firOpBuilder.getRefType(fieldTy), rec, field),
+            fieldTy};
+  };
+
+  mlir::omp::DeclareMapperInfoOperands clauseOps;
+  llvm::SmallVector<llvm::SmallVector<int64_t>> memberPlacementIndices;
+  llvm::SmallVector<mlir::Value> memberMapOps;
+
+  llvm::omp::OpenMPOffloadMappingFlags mapFlag =
+      llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO |
+      llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM |
+      llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_IMPLICIT;
+  mlir::omp::VariableCaptureKind captureKind =
+      mlir::omp::VariableCaptureKind::ByRef;
+  int64_t index = 0;
+
+  // Populate the declareMapper region with the map information.
+  for (const auto &[memberName, memberType] :
+       mlir::dyn_cast<fir::RecordType>(recordType).getTypeList()) {
+    auto [ref, type] = getFieldRef(declareOp.getBase(), index);
+    mlir::FlatSymbolRefAttr mapperId;
+    if (auto recType = mlir::dyn_cast<fir::RecordType>(memberType)) {
+      std::string mapperIdName =
+          recType.getName().str() + ".omp.default.mapper";
+      if (auto *sym = converter.getCurrentScope().FindSymbol(mapperIdName))
+        mapperIdName = converter.mangleName(mapperIdName, sym->owner());
+      else if (auto *sym = converter.getCurrentScope().FindSymbol(memberName))
+        mapperIdName = converter.mangleName(mapperIdName, sym->owner());
+
+      if (converter.getModuleOp().lookupSymbol(mapperIdName))
+        mapperId = mlir::FlatSymbolRefAttr::get(&converter.getMLIRContext(),
+                                                mapperIdName);
+      else
+        mapperId = genImplicitDefaultDeclareMapper(converter, loc, recType,
+                                                   mapperIdName);
+    }
+
+    llvm::SmallVector<mlir::Value> bounds;
+    genBoundsOps(ref, bounds);
+    mlir::Value mapOp = createMapInfoOp(
+        firOpBuilder, loc, ref, /*varPtrPtr=*/mlir::Value{}, "", bounds,
+        /*members=*/{},
+        /*membersIndex=*/mlir::ArrayAttr{},
+        static_cast<
+            std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
+            mapFlag),
+        captureKind, ref.getType(), /*partialMap=*/false, mapperId);
+    memberMapOps.emplace_back(mapOp);
+    memberPlacementIndices.emplace_back(llvm::SmallVector<int64_t>{index++});
+  }
+
+  llvm::SmallVector<mlir::Value> bounds;
+  genBoundsOps(declareOp.getOriginalBase(), bounds);
+  mlir::omp::MapInfoOp mapOp = createMapInfoOp(
+      firOpBuilder, loc, declareOp.getOriginalBase(),
+      /*varPtrPtr=*/mlir::Value(), /*name=*/"", bounds, memberMapOps,
+      firOpBuilder.create2DI64ArrayAttr(memberPlacementIndices),
+      static_cast<std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
+          mapFlag),
+      captureKind, declareOp.getType(0),
+      /*partialMap=*/true);
+
+  clauseOps.mapVars.emplace_back(mapOp);
+
+  firOpBuilder.create<mlir::omp::DeclareMapperInfoOp>(loc, clauseOps.mapVars);
+  // declMapperOp->dumpPretty();
+  return mlir::FlatSymbolRefAttr::get(&converter.getMLIRContext(),
+                                      mapperNameStr);
+}
+
 static mlir::omp::TargetOp
 genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
             lower::StatementContext &stmtCtx,
@@ -2420,15 +2538,26 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
       name << sym.name().ToString();
 
       mlir::FlatSymbolRefAttr mapperId;
-      if (sym.GetType()->category() == semantics::DeclTypeSpec::TypeDerived) {
+      if (sym.GetType()->category() == semantics::DeclTypeSpec::TypeDerived &&
+          defaultMaps.empty()) {
         auto &typeSpec = sym.GetType()->derivedTypeSpec();
         std::string mapperIdName =
             typeSpec.name().ToString() + ".omp.default.mapper";
         if (auto *sym = converter.getCurrentScope().FindSymbol(mapperIdName))
           mapperIdName = converter.mangleName(mapperIdName, sym->owner());
+        else
+          mapperIdName =
+              converter.mangleName(mapperIdName, *typeSpec.GetScope());
+
         if (converter.getModuleOp().lookupSymbol(mapperIdName))
           mapperId = mlir::FlatSymbolRefAttr::get(&converter.getMLIRContext(),
                                                   mapperIdName);
+        else
+          mapperId = genImplicitDefaultDeclareMapper(
+              converter, loc,
+              mlir::cast<fir::RecordType>(
+                  converter.genType(sym.GetType()->derivedTypeSpec())),
+              mapperIdName);
       }
 
       fir::factory::AddrAndBoundsInfo info =
@@ -4039,6 +4168,7 @@ genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
   ClauseProcessor cp(converter, semaCtx, clauses);
   cp.processMap(loc, stmtCtx, clauseOps);
   firOpBuilder.create<mlir::omp::DeclareMapperInfoOp>(loc, clauseOps.mapVars);
+  // declMapperOp->dumpPretty();
 }
 
 static void
diff --git a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
index 86095d708f7e5..1ef6239e51fa8 100644
--- a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
+++ b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
@@ -377,7 +377,7 @@ class MapInfoFinalizationPass
                                    getDescriptorMapType(mapType, target)),
             op.getMapCaptureTypeAttr(), /*varPtrPtr=*/mlir::Value{}, newMembers,
             newMembersAttr, /*bounds=*/mlir::SmallVector<mlir::Value>{},
-            /*mapperId*/ mlir::FlatSymbolRefAttr(), op.getNameAttr(),
+            op.getMapperIdAttr(), op.getNameAttr(),
             /*partial_map=*/builder.getBoolAttr(false));
     op.replaceAllUsesWith(newDescParentMapOp.getResult());
     op->erase();
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 322562b06b87f..318af99a57848 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -2163,7 +2163,7 @@ void AttrsVisitor::SetBindNameOn(Symbol &symbol) {
   }
   symbol.SetBindName(std::move(*label));
   if (!oldBindName.empty()) {
-    if (const std::string * newBindName{symbol.GetBindName()}) {
+    if (const std::string *newBindName{symbol.GetBindName()}) {
       if (oldBindName != *newBindName) {
         Say(symbol.name(),
             "The entity '%s' has multiple BIND names ('%s' and '%s')"_err_en_US,
@@ -2285,7 +2285,7 @@ void DeclTypeSpecVisitor::Post(const parser::TypeSpec &typeSpec) {
   // expression semantics if the DeclTypeSpec is a valid TypeSpec.
   // The grammar ensures that it's an intrinsic or derived type spec,
   // not TYPE(*) or CLASS(*) or CLASS(T).
-  if (const DeclTypeSpec * spec{state_.declTypeSpec}) {
+  if (const DeclTypeSpec *spec{state_.declTypeSpec}) {
     switch (spec->category()) {
     case DeclTypeSpec::Numeric:
     case DeclTypeSpec::Logical:
@@ -2293,7 +2293,7 @@ void DeclTypeSpecVisitor::Post(const parser::TypeSpec &typeSpec) {
       typeSpec.declTypeSpec = spec;
       break;
     case DeclTypeSpec::TypeDerived:
-      if (const DerivedTypeSpec * derived{spec->AsDerived()}) {
+      if (const DerivedTypeSpec *derived{spec->AsDerived()}) {
         CheckForAbstractType(derived->typeSymbol()); // C703
         typeSpec.declTypeSpec = spec;
       }
@@ -2802,8 +2802,8 @@ Symbol &ScopeHandler::MakeSymbol(const parser::Name &name, Attrs attrs) {
 Symbol &ScopeHandler::MakeHostAssocSymbol(
     const parser::Name &name, const Symbol &hostSymbol) {
   Symbol &symbol{*NonDerivedTypeScope()
-                      .try_emplace(name.source, HostAssocDetails{hostSymbol})
-                      .first->second};
+          .try_emplace(name.source, HostAssocDetails{hostSymbol})
+          .first->second};
   name.symbol = &symbol;
   symbol.attrs() = hostSymbol.attrs(); // TODO: except PRIVATE, PUBLIC?
   // These attributes can be redundantly reapplied without error
@@ -2891,7 +2891,7 @@ void ScopeHandler::ApplyImplicitRules(
   if (context().HasError(symbol) || !NeedsType(symbol)) {
     return;
   }
-  if (const DeclTypeSpec * type{GetImplicitType(symbol)}) {
+  if (const DeclTypeSpec *type{GetImplicitType(symbol)}) {
     if (!skipImplicitTyping_) {
       symbol.set(Symbol::Flag::Implicit);
       symbol.SetType(*type);
@@ -2991,7 +2991,7 @@ const DeclTypeSpec *ScopeHandler::GetImplicitType(
   const auto *type{implicitRulesMap_->at(scope).GetType(
       symbol.name(), respectImplicitNoneType)};
   if (type) {
-    if (const DerivedTypeSpec * derived{type->AsDerived()}) {
+    if (const DerivedTypeSpec *derived{type->AsDerived()}) {
       // Resolve any forward-referenced derived type; a quick no-op else.
       auto &instantiatable{*const_cast<DerivedTypeSpec *>(derived)};
       instantiatable.Instantiate(currScope());
@@ -3706,10 +3706,10 @@ void ModuleVisitor::DoAddUse(SourceName location, SourceName localName,
     } else if (IsPointer(p1) || IsPointer(p2)) {
       return false;
     } else if (const auto *subp{p1.detailsIf<SubprogramDetails>()};
-               subp && !subp->isInterface()) {
+        subp && !subp->isInterface()) {
       return false; // defined in module, not an external
     } else if (const auto *subp{p2.detailsIf<SubprogramDetails>()};
-               subp && !subp->isInterface()) {
+        subp && !subp->isInterface()) {
       return false; // defined in module, not an external
     } else {
       // Both are external interfaces, perhaps to the same procedure
@@ -3969,7 +3969,7 @@ Scope *ModuleVisitor::FindModule(const parser::Name &name,
   if (scope) {
     if (DoesScopeContain(scope, currScope())) { // 14.2.2(1)
       std::optional<SourceName> submoduleName;
-      if (const Scope * container{FindModuleOrSubmoduleContaining(currScope())};
+      if (const Scope *container{FindModuleOrSubmoduleContaining(currScope())};
           container && container->IsSubmodule()) {
         submoduleName = container->GetName();
       }
@@ -4074,7 +4074,7 @@ bool InterfaceVisitor::isAbstract() const {
 
 void InterfaceVisitor::AddSpecificProcs(
     const std::list<parser::Name> &names, ProcedureKind kind) {
-  if (Symbol * symbol{GetGenericInfo().symbol};
+  if (Symbol *symbol{GetGenericInfo().symbol};
       symbol && symbol->has<GenericDetails>()) {
     for (const auto &name : names) {
       specificsForGenericProcs_.emplace(symbol, std::make_pair(&name, kind));
@@ -4174,7 +4174,7 @@ void GenericHandler::DeclaredPossibleSpecificProc(Symbol &proc) {
 }
 
 void InterfaceVisitor::ResolveNewSpecifics() {
-  if (Symbol * generic{genericInfo_.top().symbol};
+  if (Symbol *generic{genericInfo_.top().symbol};
       generic && generic->has<GenericDetails>()) {
     ResolveSpecificsInGeneric(*generic, false);
   }
@@ -4259,7 +4259,7 @@ bool SubprogramVisitor::HandleStmtFunction(const parser::StmtFunctionStmt &x) {
           name.source);
       MakeSymbol(name, Attrs{}, UnknownDetails{});
     } else if (auto *entity{ultimate.detailsIf<EntityDetails>()};
-               entity && !ultimate.has<ProcEntityDetails>()) {
+        entity && !ultimate.has<ProcEntityDetails>()) {
       resultType = entity->type();
       ultimate.details() = UnknownDetails{}; // will be replaced below
     } else {
@@ -4315,7 +4315,7 @@ bool SubprogramVisitor::Pre(const parser::Suffix &suffix) {
     } else {
       Message &msg{Say(*suffix.resultName,
           "RESULT(%s) may appear only in a function"_err_en_US)};
-      if (const Symbol * subprogram{InclusiveScope().symbol()}) {
+      if (const Symbol *subprogram{InclusiveScope().symbol()}) {
         msg.Attach(subprogram->name(), "Containing subprogram"_en_US);
       }
     }
@@ -4833,7 +4833,7 @@ Symbol *ScopeHandler::FindSeparateModuleProcedureInterface(
       symbol = generic->specific();
     }
   }
-  if (const Symbol * defnIface{FindSeparateModuleSubprogramInterface(symbol)}) {
+  if (const Symbol *defnIface{FindSeparateModuleSubprogramInterface(symbol)}) {
     // Error recovery in case of multiple definitions
     symbol = const_cast<Symbol *>(defnIface);
   }
@@ -4970,8 +4970,8 @@ bool SubprogramVisitor::HandlePreviousCalls(
     return generic->specific() &&
         HandlePreviousCalls(name, *generic->specific(), subpFlag);
   } else if (const auto *proc{symbol.detailsIf<ProcEntityDetails>()}; proc &&
-             !proc->isDummy() &&
-             !symbol.attrs().HasAny(Attrs{Attr::INTRINSIC, Attr::POINTER})) {
+      !proc->isDummy() &&
+      !symbol.attrs().HasAny(Attrs{Attr::INTRINSIC, Attr::POINTER})) {
     // There's a symbol created for previous calls to this subprogram or
     // ENTRY's name.  We have to replace that symbol in situ to avoid the
     // obligation to rewrite symbol pointers in the parse tree.
@@ -5012,7 +5012,7 @@ void SubprogramVisitor::CheckExtantProc(
   if (auto *prev{FindSymbol(name)}) {
     if (IsDummy(*prev)) {
     } else if (auto *entity{prev->detailsIf<EntityDetails>()};
-               IsPointer(*prev) && entity && !entity->type()) {
+        IsPointer(*prev) && entity && !entity->type()) {
       // POINTER attribute set before interface
     } else if (inInterfaceBlock() && currScope() != prev->owner()) {
       // Procedures in an INTERFACE block do not resolve to symbols
@@ -5068,7 +5068,7 @@ Symbol &SubprogramVisitor::PushSubprogramScope(const parser::Name &name,
     }
     set_inheritFromParent(false); // interfaces don't inherit, even if MODULE
   }
-  if (Symbol * found{FindSymbol(name)};
+  if (Symbol *found{FindSymbol(name)};
       found && found->has<HostAssocDetails>()) {
     found->set(subpFlag); // PushScope() created symbol
   }
@@ -5910,9 +5910,9 @@ void DeclarationVisitor::Post(const parser::VectorTypeSpec &x) {
     vectorDerivedType.CookParameters(GetFoldingContext());
   }
 
-  if (const DeclTypeSpec *
-      extant{ppcBuiltinTypesScope->FindInstantiatedDerivedType(
-          vectorDerivedType, DeclTypeSpec::Category::TypeDerived)}) {
+  if (const DeclTypeSpec *extant{
+          ppcBuiltinTypesScope->FindInstantiatedDerivedType(
+              vectorDerivedType, DeclTypeSpec::Category::TypeDerived)}) {
     // This derived type and parameter expressions (if any) are already present
     // in the __ppc_intrinsics scope.
     SetDeclTypeSpec(*extant);
@@ -5934,7 +5934,7 @@ bool DeclarationVisitor::Pre(const parser::DeclarationTypeSpec::Type &) {
 
 void DeclarationVisitor::Post(const parser::DeclarationTypeSpec::Type &type) {
   const parser::Name &derivedName{std::get<parser::Name>(type.derived.t)};
-  if (const Symbol * derivedSymbol{derivedName.symbol}) {
+  if (const Symbol *derivedSymbol{derivedName.symbol}) {
     CheckForAbstractType(*derivedSymbol); // C706
   }
 }
@@ -6003,8 +6003,8 @@ void DeclarationVisitor::Post(const parser::DerivedTypeSpec &x) {
   if (!spec->MightBeParameterized()) {
     spec->EvaluateParameters(context());
   }
-  if (const DeclTypeSpec *
-      extant{currScope().FindInstantiatedDerivedType(*spec, category)}) {
+  if (const DeclTypeSpec *extant{
+          currScope().FindInstantiatedDerivedType(*spec, category)}) {
     // This derived type and parameter expressions (if any) are already present
     // in this scope.
     SetDeclTypeSpec(*extant);
@@ -6035,8 +6035,7 @@ void DeclarationVisitor::Post(const parser::DeclarationTypeSpec::Record &rec) {
   if (auto spec{ResolveDerivedType(typeName)}) {
     spec->CookParameters(GetFoldingContext());
   ...
[truncated]

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements implicit generation of default OpenMP declare mappers for derived types (including allocatable ones) when no user-supplied mapper is provided, updating tests and lowering passes to exercise and emit these mappers.

  • Added FileCheck tests for implicit declare mappers in derived-type-map.f90
  • Introduced genImplicitDefaultDeclareMapper in the OpenMP lowering to emit DeclareMapperOp regions
  • Wired through the new mapper IDs in the map info finalization and clause processing

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
flang/test/Lower/OpenMP/derived-type-map.f90 Added CHECK lines for implicit declare mappers
flang/lib/Semantics/resolve-names.cpp Minor brace-formatting cleanup
flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp Use the real mapperIdAttr instead of a blank attr
flang/lib/Lower/OpenMP/OpenMP.cpp Added genImplicitDefaultDeclareMapper and hook up mapper IDs
flang/lib/Lower/OpenMP/ClauseProcessor.cpp Adjusted default-mapper lookup and avoid self-mapping
Comments suppressed due to low confidence (2)

flang/test/Lower/OpenMP/derived-type-map.f90:23

  • The uniq_name uses Escalar instead of Scalar. Correcting this typo will keep naming consistent with the derived type scalar_and_array.
!CHECK: %[[ALLOCA:.*]] = fir.alloca !fir.box<!fir.heap<!fir.type<_QFmaptype_derived_implicit_allocatableTscalar_and_array{real:f32,array:!fir.array<10xi32>,int:i32}>>> {bindc_name = "scalar_arr", uniq_name = "_QFmaptype_derived_implicit_allocatableEscalar_arr"}

flang/lib/Lower/OpenMP/OpenMP.cpp:2351

  • [nitpick] Consider adding a brief comment above genImplicitDefaultDeclareMapper to explain its purpose, inputs, and outputs to improve readability and maintainability.
static mlir::FlatSymbolRefAttr

Copy link

github-actions bot commented May 19, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@TIFitis TIFitis force-pushed the users/Akash/implicit_default_mapper branch from 5d735f1 to 580e862 Compare May 19, 2025 15:41
Base automatically changed from users/Akash/mapper_semantic to main May 28, 2025 13:32
Copy link
Member

@skatrak skatrak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you Akash for this work, and excuse the delay!

@TIFitis TIFitis force-pushed the users/Akash/implicit_default_mapper branch from 99a489d to 138250f Compare July 2, 2025 16:43
Copy link
Member

@skatrak skatrak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you Akash for addressing my comments, and please excuse the delay taking a second look! LGTM.

Copy link
Contributor

@agozillon agozillon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Akash, this LGTM.

However, could you please add a test to the map-types-and-sizes.f90 test or create an equivalent for the mapper work! Would be good to keep track of what we expect the lowered LLVM-IR to be.

I also don't believe this PR handles more complicated mapping cases like below (please do correct me if I am wrong):

program main
    type :: bottom_layer
      integer(4), allocatable :: array_j3(:)
    end type bottom_layer

    type :: middle_layer
     type(bottom_layer), allocatable :: nest
     real(4), allocatable :: array_j2(:)
    end type middle_layer

    type :: top_layer
    integer, allocatable :: array_j(:)
    type(middle_layer), allocatable :: nested
    end type top_layer

    type(top_layer), allocatable :: top_dtype

    allocate(top_dtype)
    allocate(top_dtype%nested)
    allocate(top_dtype%nested%nest)
    allocate(top_dtype%array_j(10))
    allocate(top_dtype%nested%array_j2(10))
    allocate(top_dtype%nested%nest%array_j3(10))

!$omp target
  top_dtype%array_j(1) = 10
  top_dtype%nested%array_j2(1) = 20
  top_dtype%nested%nest%array_j3(1) = 30
!$omp end target

  print *, top_dtype%nested%nest%array_j3(1)
  print *, top_dtype%nested%array_j2(1)
  print *, top_dtype%array_j(1)

end program main

How difficult would it be to extend this work to handle the above cases? As I think that'd be the ideal next step for this work!

A really nice end goal for this work would be to handle slightly more complicated case than even the above with arbitrary nestings of array of allocatable derived types with allocatable types (arrays, scalars etc.)!

/*contiguousHint=*/true)
.first;
fir::factory::AddrAndBoundsInfo info = fir::factory::getDataOperandBaseAddr(
firOpBuilder, mapVal, /*isOptional=*/false, mapVal.getLoc());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this would break if the input record type to a function was optional and we opted to not provide the argument, ran into a similar issue a while ago. Not the biggest deal for this PR, but good idea to test it at some point!

…ypes

This patch adds support to emit default declare mappers for implicit mapping of derived types when not supplied by user. This especially helps tackle mapping of allocatables of derived types.
This supports nested derived types as well.
@TIFitis TIFitis force-pushed the users/Akash/implicit_default_mapper branch from c2ca1e8 to 9dded5e Compare July 28, 2025 19:38
boomanaiden154 and others added 3 commits July 28, 2025 20:39
This patch changes when we install the GHA runner in the CI containers. Instead
of having it in the base image, we install it last. This will enable a follow up
patch that will do some setup enabling building the full container image with an
existing base image, thus enabling updating the GHA runner without modifying the
important bits.

Reviewers: EricWF, ldionne

Reviewed By: ldionne

Pull Request: #148072
This patch does some refactoring to enable installing a new GHA runner binary
into an existing libcxx image. We achieve this by pushing the base image to the
registry and enabling control over the base image used for building the actions
image. This will always build and push both images even if an existing image is
being used for the actions image, but this should not impact anything as the
SHAs are pinned everywhere and space/build time is not a large concern.

Reviewers: ldionne, EricWF, #reviewers-libcxx

Reviewed By: ldionne

Pull Request: #148073
@TIFitis TIFitis requested a review from a team as a code owner July 28, 2025 23:23
@llvmbot llvmbot added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. github:workflow labels Jul 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang:openmp flang:semantics flang Flang issues not falling into any other category github:workflow libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants